Add OpenAPI 3.0 specification for API v2#57
Conversation
Provide a machine-readable OpenAPI spec covering all 9 v2 endpoints with request/response schemas, examples, and error definitions. Update api-v2.md to reference the new spec file. Closes #54
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the API documentation by introducing a machine-readable OpenAPI 3.0 specification for the v2 API. This addition will improve the developer experience by providing a standardized and easily consumable format for understanding and integrating with the API, enabling automated client generation and interactive documentation tools. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThis change adds comprehensive API documentation by creating a new OpenAPI 3.0.3 specification file for Orchestrator API v2 and adding a reference to it in the existing API documentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a comprehensive OpenAPI 3.0 specification for the API v2, significantly improving API documentation and enabling better tooling integration. The docs/api-v2.md file has been updated to reference this new specification, enhancing discoverability for users. The new OpenAPI definition covers all nine v2 API endpoints with detailed request parameters, response schemas, examples, and error definitions, which is a great step towards a more robust and user-friendly API.
| items: | ||
| type: string | ||
| RecoveryStartTimestamp: | ||
| type: string |
There was a problem hiding this comment.
For RecoveryStartTimestamp, consider using the format: date-time property and updating the example to an ISO 8601 compliant string (e.g., "2025-01-15T10:30:00Z"). This provides clearer guidance for API consumers and improves compatibility with OpenAPI tooling that expects standard date-time formats.
RecoveryStartTimestamp:
type: string
format: date-time
description: Timestamp when recovery started.
example: "2025-01-15T10:30:00Z"| description: Timestamp when recovery started. | ||
| example: "2025-01-15 10:30:00" | ||
| RecoveryEndTimestamp: | ||
| type: string |
There was a problem hiding this comment.
Similar to RecoveryStartTimestamp, it would be beneficial to use format: date-time for RecoveryEndTimestamp and provide an ISO 8601 compliant example. This ensures consistency and adherence to common API best practices for date-time representation.
RecoveryEndTimestamp:
type: string
format: date-time
description: Timestamp when recovery ended.
example: "2025-01-15T10:30:45Z"| type: object | ||
| properties: | ||
| Hostname: | ||
| type: string | ||
| example: "orchestrator-1" |
There was a problem hiding this comment.
The ActiveNode schema in HealthStatus currently only includes Hostname. However, the Go process.NodeHealth struct (which ActiveNode represents) contains additional fields like Token and AppVersion. If these fields are relevant for API consumers, consider expanding the ActiveNode schema to include them for a more complete representation of the active node's health information.
ActiveNode:
type: object
properties:
Hostname:
type: string
example: "orchestrator-1"
Token:
type: string
example: "abc123"
AppVersion:
type: string
example: "2.0.0"There was a problem hiding this comment.
Pull request overview
Adds an OpenAPI 3.0 specification for Orchestrator API v2 and links it from the existing v2 API documentation, enabling easier client generation and interactive API browsing.
Changes:
- Added
docs/api/openapi.yamldocumenting the 9/api/v2endpoints, including schemas and examples. - Updated
docs/api-v2.mdto reference the new OpenAPI spec.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/api/openapi.yaml | Introduces an OpenAPI 3.0.3 spec for all API v2 endpoints, including shared response envelope and component schemas. |
| docs/api-v2.md | Adds a short “OpenAPI Specification” section linking to the new spec file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| example: | ||
| status: error | ||
| error: | ||
| code: INTERNAL_ERROR | ||
| message: "An unexpected error occurred" |
There was a problem hiding this comment.
The shared InternalServerError response example uses error code INTERNAL_ERROR, but the v2 handlers return endpoint-specific 500 codes (e.g. CLUSTER_LIST_ERROR, INSTANCE_READ_ERROR, TOPOLOGY_ERROR, RECOVERIES_ERROR, PROXYSQL_ERROR, UNHEALTHY). This makes the spec misleading for clients that rely on error.code. Suggest either (a) making the shared 500 response example/code generic (or omitting the example), or (b) defining per-endpoint 500 responses/examples with the actual codes returned by each handler.
| example: | |
| status: error | |
| error: | |
| code: INTERNAL_ERROR | |
| message: "An unexpected error occurred" |
| - url: /api/v2 | ||
| description: Default (relative to orchestrator base URL, respects URLPrefix) |
There was a problem hiding this comment.
servers[0].url is set to /api/v2, but the actual router mounts v2 under config.Config.URLPrefix + "/api/v2" (see go/http/apiv2.go). As written, the generated clients/docs will be wrong when URLPrefix is non-empty. Consider using a server variable like {basePath}/api/v2 (with basePath defaulting to "" and documented as the configured URLPrefix), or listing two server entries to cover both cases.
| - url: /api/v2 | |
| description: Default (relative to orchestrator base URL, respects URLPrefix) | |
| - url: "{basePath}/api/v2" | |
| description: Default v2 API base path. `basePath` should match the configured `URLPrefix` (empty if not set). | |
| variables: | |
| basePath: | |
| default: "" | |
| description: Base path prefix matching orchestrator config `URLPrefix` (leave empty if URLPrefix is not set). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/api/openapi.yaml (2)
14-16: Consider adding explicit security documentation.Static analysis flags missing security definitions (CKV_OPENAPI_4, CKV_OPENAPI_5). Even if the API intentionally requires no authentication (or relies on network-level security), it's best practice to document this explicitly:
♻️ Proposed fix to add explicit security declaration
servers: - url: /api/v2 description: Default (relative to orchestrator base URL, respects URLPrefix) + +security: [] # No authentication required; access controlled at network levelIf authentication is planned or exists at another layer, consider adding the appropriate
securitySchemesincomponentsinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api/openapi.yaml` around lines 14 - 16, The OpenAPI spec currently lacks explicit security declarations; update docs/api/openapi.yaml to include either a components.securitySchemes entry describing the auth method(s) used (e.g., bearerAuth, apiKey) and add a top-level security requirement, or explicitly document no authentication by adding an empty security: [] at the root and a clear comment under components explaining network-level/no-auth assumptions; modify the file near the existing servers: block and add components.securitySchemes and security entries so static analysis (CKV_OPENAPI_4/5) no longer flags missing security.
36-39: Consider addingmaxItemsconstraints to array schemas.Static analysis (CKV_OPENAPI_21) flags that array types lack
maxItemsbounds. While not critical for documentation, adding limits can help API consumers understand expected cardinality and enables better client-side validation.♻️ Example fix for the clusters array
data: type: array + maxItems: 1000 items: $ref: "#/components/schemas/ClusterInfo"This pattern could be applied to other arrays (
instances,recoveries,RaftHealthyMembers, etc.) based on realistic operational limits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api/openapi.yaml` around lines 36 - 39, Add a maxItems constraint to the array schema that currently defines "data" with items: $ref "#/components/schemas/ClusterInfo" by updating that schema to include a sensible maxItems value (e.g., based on expected cluster size) so clients and validators have an upper bound; apply the same pattern to other array schemas such as those named "instances", "recoveries", and "RaftHealthyMembers" where realistic operational limits exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/api/openapi.yaml`:
- Around line 14-16: The OpenAPI spec currently lacks explicit security
declarations; update docs/api/openapi.yaml to include either a
components.securitySchemes entry describing the auth method(s) used (e.g.,
bearerAuth, apiKey) and add a top-level security requirement, or explicitly
document no authentication by adding an empty security: [] at the root and a
clear comment under components explaining network-level/no-auth assumptions;
modify the file near the existing servers: block and add
components.securitySchemes and security entries so static analysis
(CKV_OPENAPI_4/5) no longer flags missing security.
- Around line 36-39: Add a maxItems constraint to the array schema that
currently defines "data" with items: $ref "#/components/schemas/ClusterInfo" by
updating that schema to include a sensible maxItems value (e.g., based on
expected cluster size) so clients and validators have an upper bound; apply the
same pattern to other array schemas such as those named "instances",
"recoveries", and "RaftHealthyMembers" where realistic operational limits exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0598fbf9-549c-4f09-9ca0-e319d3902882
📒 Files selected for processing (2)
docs/api-v2.mddocs/api/openapi.yaml
Summary
docs/api/openapi.yamlcovering all 9 v2 API endpoints with request parameters, response schemas, examples, and error definitions.docs/api-v2.mdto reference the new OpenAPI spec file.Closes #54
Test plan
python3 -c "import yaml; yaml.safe_load(open('docs/api/openapi.yaml'))"go/http/apiv2.goSummary by CodeRabbit
Documentation